Fix/mot17 issues 447 449 450#561
Conversation
Adds parallel execution of test cases to address the unbearable time overhead when testing multiple hyperparameter groups sequentially (issue 8). Changes: - testcasecontroller.py: add parallel/max_workers params to run_testcases(), add _run_testcases_sequential() and _run_testcases_parallel() using ThreadPoolExecutor - testcase.py: fix race condition in _get_output_dir() using os.makedirs(exist_ok=True) instead of os.path.exists() loop - benchmarkingjob.py: add parallel and max_workers config fields - docs/proposals/chore/parallel_testcase_execution.md: RFC proposing phased approach (ThreadPoolExecutor now, Sandbox Engine future per PR 526) ThreadPoolExecutor chosen over ProcessPoolExecutor to avoid pickling issues with ML model objects. Parallel execution is opt-in via benchmarkingjob.yaml (parallel: false by default) for full backward compatibility. Signed-off-by: dhruvraj319 <dhruv.raj319@gmail.com>
- Remove manual worker count default to avoid OOM on large test suites — let ThreadPoolExecutor use its own safe default - Fix type annotation: max_workers: int | None = None - Add YAML string coercion for parallel and max_workers params to handle parallel: 'false' string values correctly - Deep copy test cases in parallel mode to prevent race conditions from shared test_env and dataset state Signed-off-by: dhruvraj319 <dhruv.raj319@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rajdhruvsingh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubeedge-bot
added
the
size/L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces parallel test case execution using ThreadPoolExecutor, resolves a directory creation race condition, adds lazy importing for onnx, and fixes evaluation bugs in the MOT17 pedestrian tracking example when queries have no matching gallery identities. The feedback highlights several critical issues: the use of PEP 604 union types (int|None) which breaks backward compatibility with Python < 3.10, a potential division-by-zero crash and incorrect denominator calculation in the CMC metrics (rank_1.py, rank_2.py, rank_5.py), and potential memory/CPU overhead from deep-copying entire TestCase objects during parallel execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…multiedge_inference_bench - fix(447): guard np.nonzero()[0][0] in rank_1/2/5.py to skip queries with no gallery match instead of raising IndexError - fix(447): replace RuntimeError in mAP.py with LOGGER.warning + return 0.0 so the pipeline degrades gracefully on empty query sets - fix(449): add requirements.txt with all missing dependencies (loguru, motmetrics, mmcv, opencv-python) and version bounds (motmetrics>=1.2.0, pandas<2.0) - fix(449): replace Docker-only absolute workspace path /ianvs/... with relative path ./workspace/... in tracking_job.yaml and reid_job.yaml - fix(450): move top-level 'import onnx' to lazy import inside _partition() so non-partitioning users (MOT17 example) are not forced to install onnx Signed-off-by: dhruvraj319 <dhruv.raj319@gmail.com>
rajdhruvsingh
force-pushed
the
fix/mot17-issues-447-449-450
branch
from
a70209d to
8479182
Compare
2 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
Fixes three bugs in the
MOT17/multiedge_inference_benchexample with no existing linked PRs, as tracked in the issue summarization #460.#447 — IndexError / RuntimeError in ReID metric scripts (
rank_1.py,rank_2.py,rank_5.py,mAP.py)The CMC implementation used
np.nonzero(matches[i])[0][0]without a lengthcheck, causing an
IndexErrorwhen a query identity has no match in thegallery — a valid edge case in standard ReID evaluation splits. Fixed by
checking
len(nonzero) == 0before indexing and skipping such queries.mAP.pyraised an unhandledRuntimeError("No valid query")when the entirequery set was unmatched. Replaced with a
LOGGER.warningand a gracefulreturn 0.0so the pipeline continues and surfaces a clear diagnostic messageinstead of crashing.
#449 — Missing
requirements.txtand Docker-only hardcoded workspace pathsThe example had no
requirements.txt, forcing contributors to discover missingpackages (
loguru,motmetrics,mmcv,opencv-python) through repeatedImportErrorcrashes. Added arequirements.txtwith all required packagesand version bounds (
motmetrics>=1.2.0,pandas<2.0).Both
tracking_job.yamlandreid_job.yamlhardcoded the workspace path as/ianvs/multiedge_inference_bench/workspace, which only exists inside theofficial ianvs Docker container and causes
PermissionErrororFileNotFoundErrorfor all bare-metal and virtualenv users. Replaced with therelative path
./workspace/multiedge_inference_bench.#450 — Unconditional
import onnxat module scope inmultiedge_inference.pyonnxwas imported unconditionally at the top ofmultiedge_inference.py,making it a hard startup dependency for all
MultiedgeInferenceusers eventhough it is only required inside
_partition(). The MOT17 example never calls_partition(). Traced viagit log -p -- requirements.txt, the currentonnxentry in the rootrequirements.txtwas added incidentally by commit46340c8("Enhanced cloud-edge collaborative inference for llm example") — notas a deliberate decision for this paradigm. If example-level dependencies are
ever isolated per issue #132, this would regress. Fixed by moving
import onnxto a lazy import inside
_partition()only, with a clearImportErrormessagefor users who attempt to use partitioning without it installed.
Which issue(s) this PR fixes:
Fixes #447
Fixes #449
Fixes #450